From 0cc39aaef919bf8d32779d85c2948ca4d9fd39d6 Mon Sep 17 00:00:00 2001 From: Alex Crichton Date: Mon, 21 Nov 2016 10:05:55 -0800 Subject: [PATCH] Refactor metadata generation Remove generation all the way in manifest-parsing and defer it until we actually need it during compilation. Additionally remove lots of weird logic that's no longer necessary that we're hashing quite a few fields. --- src/cargo/core/manifest.rs | 27 ++----- src/cargo/core/mod.rs | 2 +- src/cargo/core/package.rs | 6 +- src/cargo/core/package_id.rs | 23 +----- src/cargo/ops/cargo_clean.rs | 5 +- src/cargo/ops/cargo_rustc/context.rs | 91 ++++++++++++------------ src/cargo/ops/cargo_rustc/fingerprint.rs | 4 +- src/cargo/ops/cargo_rustc/mod.rs | 17 +++-- src/cargo/util/toml.rs | 54 ++++---------- 9 files changed, 81 insertions(+), 148 deletions(-) diff --git a/src/cargo/core/manifest.rs b/src/cargo/core/manifest.rs index f5f4217da..93fac21fd 100644 --- a/src/cargo/core/manifest.rs +++ b/src/cargo/core/manifest.rs @@ -6,7 +6,6 @@ use rustc_serialize::{Encoder, Encodable}; use core::{Dependency, PackageId, Summary, SourceId, PackageIdSpec}; use core::WorkspaceConfig; -use core::package_id::Metadata; pub enum EitherManifest { Real(Manifest), @@ -159,7 +158,6 @@ pub struct Target { kind: TargetKind, name: String, src_path: PathBuf, - metadata: Option, tested: bool, benched: bool, doc: bool, @@ -279,7 +277,6 @@ impl Target { kind: TargetKind::Bin, name: String::new(), src_path: PathBuf::new(), - metadata: None, doc: false, doctest: false, harness: true, @@ -289,40 +286,35 @@ impl Target { } } - pub fn lib_target(name: &str, crate_targets: Vec, - src_path: &Path, - metadata: Metadata) -> Target { + pub fn lib_target(name: &str, + crate_targets: Vec, + src_path: &Path) -> Target { Target { kind: TargetKind::Lib(crate_targets), name: name.to_string(), src_path: src_path.to_path_buf(), - metadata: Some(metadata), doctest: true, doc: true, ..Target::blank() } } - pub fn bin_target(name: &str, src_path: &Path, - metadata: Option) -> Target { + pub fn bin_target(name: &str, src_path: &Path) -> Target { Target { kind: TargetKind::Bin, name: name.to_string(), src_path: src_path.to_path_buf(), - metadata: metadata, doc: true, ..Target::blank() } } /// Builds a `Target` corresponding to the `build = "build.rs"` entry. - pub fn custom_build_target(name: &str, src_path: &Path, - metadata: Option) -> Target { + pub fn custom_build_target(name: &str, src_path: &Path) -> Target { Target { kind: TargetKind::CustomBuild, name: name.to_string(), src_path: src_path.to_path_buf(), - metadata: metadata, for_host: true, benched: false, tested: false, @@ -340,25 +332,21 @@ impl Target { } } - pub fn test_target(name: &str, src_path: &Path, - metadata: Metadata) -> Target { + pub fn test_target(name: &str, src_path: &Path) -> Target { Target { kind: TargetKind::Test, name: name.to_string(), src_path: src_path.to_path_buf(), - metadata: Some(metadata), benched: false, ..Target::blank() } } - pub fn bench_target(name: &str, src_path: &Path, - metadata: Metadata) -> Target { + pub fn bench_target(name: &str, src_path: &Path) -> Target { Target { kind: TargetKind::Bench, name: name.to_string(), src_path: src_path.to_path_buf(), - metadata: Some(metadata), tested: false, ..Target::blank() } @@ -367,7 +355,6 @@ impl Target { pub fn name(&self) -> &str { &self.name } pub fn crate_name(&self) -> String { self.name.replace("-", "_") } pub fn src_path(&self) -> &Path { &self.src_path } - pub fn metadata(&self) -> Option<&Metadata> { self.metadata.as_ref() } pub fn kind(&self) -> &TargetKind { &self.kind } pub fn tested(&self) -> bool { self.tested } pub fn harness(&self) -> bool { self.harness } diff --git a/src/cargo/core/mod.rs b/src/cargo/core/mod.rs index 305b6ed7c..e663dcdf2 100644 --- a/src/cargo/core/mod.rs +++ b/src/cargo/core/mod.rs @@ -2,7 +2,7 @@ pub use self::dependency::{Dependency, DependencyInner}; pub use self::manifest::{Manifest, Target, TargetKind, Profile, LibKind, Profiles}; pub use self::manifest::{EitherManifest, VirtualManifest}; pub use self::package::{Package, PackageSet}; -pub use self::package_id::{PackageId, Metadata}; +pub use self::package_id::PackageId; pub use self::package_id_spec::PackageIdSpec; pub use self::registry::Registry; pub use self::resolver::Resolve; diff --git a/src/cargo/core/package.rs b/src/cargo/core/package.rs index 03ed7d887..11e53cf2c 100644 --- a/src/cargo/core/package.rs +++ b/src/cargo/core/package.rs @@ -7,7 +7,7 @@ use std::path::{Path, PathBuf}; use semver::Version; use core::{Dependency, Manifest, PackageId, SourceId, Target, TargetKind}; -use core::{Summary, Metadata, SourceMap}; +use core::{Summary, SourceMap}; use ops; use util::{CargoResult, Config, LazyCell, ChainError, internal, human, lev_distance}; use rustc_serialize::{Encoder,Encodable}; @@ -94,10 +94,6 @@ impl Package { self.targets().iter().any(|t| t.is_custom_build()) } - pub fn generate_metadata(&self) -> Metadata { - self.package_id().generate_metadata() - } - pub fn find_closest_target(&self, target: &str, kind: TargetKind) -> Option<&Target> { let targets = self.targets(); diff --git a/src/cargo/core/package_id.rs b/src/cargo/core/package_id.rs index 8f1992a73..d9224bae1 100644 --- a/src/cargo/core/package_id.rs +++ b/src/cargo/core/package_id.rs @@ -9,7 +9,7 @@ use regex::Regex; use rustc_serialize::{Encodable, Encoder, Decodable, Decoder}; use semver; -use util::{CargoResult, CargoError, short_hash, ToSemver}; +use util::{CargoResult, CargoError, ToSemver}; use core::source::SourceId; /// Identifier for a specific version of a package in a specific source. @@ -118,12 +118,6 @@ impl From for Box { fn from(t: PackageIdError) -> Box { Box::new(t) } } -#[derive(PartialEq, Eq, Hash, Clone, RustcEncodable, Debug)] -pub struct Metadata { - pub metadata: String, - pub extra_filename: String -} - impl PackageId { pub fn new(name: &str, version: T, sid: &SourceId) -> CargoResult { @@ -141,13 +135,6 @@ impl PackageId { pub fn version(&self) -> &semver::Version { &self.inner.version } pub fn source_id(&self) -> &SourceId { &self.inner.source_id } - pub fn generate_metadata(&self) -> Metadata { - let metadata = short_hash(self); - let extra_filename = format!("-{}", metadata); - - Metadata { metadata: metadata, extra_filename: extra_filename } - } - pub fn with_precise(&self, precise: Option) -> PackageId { PackageId { inner: Arc::new(PackageIdInner { @@ -169,14 +156,6 @@ impl PackageId { } } -impl Metadata { - pub fn mix(&mut self, t: &T) { - let new_metadata = short_hash(&(&self.metadata, t)); - self.extra_filename = format!("-{}", new_metadata); - self.metadata = new_metadata; - } -} - impl fmt::Display for PackageId { fn fmt(&self, f: &mut Formatter) -> fmt::Result { write!(f, "{} v{}", self.inner.name, self.inner.version)?; diff --git a/src/cargo/ops/cargo_clean.rs b/src/cargo/ops/cargo_clean.rs index 99b45f7c2..08f9b78c1 100644 --- a/src/cargo/ops/cargo_clean.rs +++ b/src/cargo/ops/cargo_clean.rs @@ -73,9 +73,8 @@ pub fn clean(ws: &Workspace, opts: &CleanOptions) -> CargoResult<()> { cx.probe_target_info(&units)?; for unit in units.iter() { - let layout = cx.layout(unit); - rm_rf(&layout.proxy().fingerprint(&unit.pkg))?; - rm_rf(&layout.build(&unit.pkg))?; + rm_rf(&cx.layout(unit).proxy().fingerprint(&unit.pkg))?; + rm_rf(&cx.layout(unit).build(&unit.pkg))?; for (src, link_dst, _) in cx.target_filenames(&unit)? { rm_rf(&src)?; diff --git a/src/cargo/ops/cargo_rustc/context.rs b/src/cargo/ops/cargo_rustc/context.rs index d95b7ee75..0d4dc8aef 100644 --- a/src/cargo/ops/cargo_rustc/context.rs +++ b/src/cargo/ops/cargo_rustc/context.rs @@ -1,12 +1,15 @@ +#![allow(deprecated)] + use std::collections::{HashSet, HashMap, BTreeSet}; use std::env; +use std::fmt; +use std::hash::{Hasher, Hash, SipHasher}; use std::path::{Path, PathBuf}; use std::str::{self, FromStr}; use std::sync::Arc; - use core::{Package, PackageId, PackageSet, Resolve, Target, Profile}; -use core::{TargetKind, Profiles, Metadata, Dependency, Workspace}; +use core::{TargetKind, Profiles, Dependency, Workspace}; use core::dependency::Kind as DepKind; use util::{CargoResult, ChainError, internal, Config, profile, Cfg, human}; @@ -53,6 +56,9 @@ struct TargetInfo { cfg: Option>, } +#[derive(Clone)] +pub struct Metadata(u64); + impl<'a, 'cfg> Context<'a, 'cfg> { pub fn new(ws: &Workspace<'cfg>, resolve: &'a Resolve, @@ -311,7 +317,7 @@ impl<'a, 'cfg> Context<'a, 'cfg> { /// We build to the path: "{filename}-{target_metadata}" /// We use a linking step to link/copy to a predictable filename /// like `target/debug/libfoo.{a,so,rlib}` and such. - pub fn target_metadata(&self, unit: &Unit) -> Option { + pub fn target_metadata(&mut self, unit: &Unit) -> Option { // No metadata for dylibs because of a couple issues // - OSX encodes the dylib name in the executable // - Windows rustc multiple files of which we can't easily link all of them @@ -336,56 +342,41 @@ impl<'a, 'cfg> Context<'a, 'cfg> { return None; } - let metadata = unit.target.metadata().cloned().map(|mut m| { - if let Some(features) = self.resolve.features(unit.pkg.package_id()) { - let mut feat_vec: Vec<&String> = features.iter().collect(); - feat_vec.sort(); - for feat in feat_vec { - m.mix(feat); - } - } - m.mix(unit.profile); - m - }); - let mut pkg_metadata = { - let mut m = unit.pkg.generate_metadata(); - if let Some(features) = self.resolve.features(unit.pkg.package_id()) { + let mut hasher = SipHasher::new_with_keys(0, 0); + + // Unique metadata per (name, source, version) triple. This'll allow us + // to pull crates from anywhere w/o worrying about conflicts + unit.pkg.package_id().hash(&mut hasher); + + // Also mix in enabled features to our metadata. This'll ensure that + // when changing feature sets each lib is separately cached. + match self.resolve.features(unit.pkg.package_id()) { + Some(features) => { let mut feat_vec: Vec<&String> = features.iter().collect(); feat_vec.sort(); - for feat in feat_vec { - m.mix(feat); - } + feat_vec.hash(&mut hasher); } - m.mix(unit.profile); - m - }; - - if unit.target.is_lib() && unit.profile.test { - // Libs and their tests are built in parallel, so we need to make - // sure that their metadata is different. - metadata.map(|mut m| { - m.mix(&"test"); - m - }) - } else if unit.target.is_bin() && unit.profile.test { - // Make sure that the name of this test executable doesn't - // conflict with a library that has the same name and is - // being tested - pkg_metadata.mix(&format!("bin-{}", unit.target.name())); - Some(pkg_metadata) - } else if unit.pkg.package_id().source_id().is_path() && - !unit.profile.test { - Some(pkg_metadata) - } else { - metadata + None => Vec::<&String>::new().hash(&mut hasher), } + + // Throw in the profile we're compiling with. This helps caching + // panic=abort and panic=unwind artifacts, additionally with various + // settings like debuginfo and whatnot. + unit.profile.hash(&mut hasher); + + // Finally throw in the target name/kind. This ensures that concurrent + // compiles of targets in the same crate don't collide. + unit.target.name().hash(&mut hasher); + unit.target.kind().hash(&mut hasher); + + Some(Metadata(hasher.finish())) } /// Returns the file stem for a given target/profile combo (with metadata) - pub fn file_stem(&self, unit: &Unit) -> String { + pub fn file_stem(&mut self, unit: &Unit) -> String { match self.target_metadata(unit) { - Some(ref metadata) => format!("{}{}", unit.target.crate_name(), - metadata.extra_filename), + Some(ref metadata) => format!("{}-{}", unit.target.crate_name(), + metadata), None => self.bin_stem(unit), } } @@ -407,7 +398,7 @@ impl<'a, 'cfg> Context<'a, 'cfg> { /// Returns an Option because in some cases we don't want to link /// (eg a dependent lib) - pub fn link_stem(&self, unit: &Unit) -> Option<(PathBuf, String)> { + pub fn link_stem(&mut self, unit: &Unit) -> Option<(PathBuf, String)> { let src_dir = self.out_dir(unit); let bin_stem = self.bin_stem(unit); let file_stem = self.file_stem(unit); @@ -441,7 +432,7 @@ impl<'a, 'cfg> Context<'a, 'cfg> { /// filename: filename rustc compiles to. (Often has metadata suffix). /// link_dst: Optional file to link/copy the result to (without metadata suffix) /// linkable: Whether possible to link against file (eg it's a library) - pub fn target_filenames(&self, unit: &Unit) + pub fn target_filenames(&mut self, unit: &Unit) -> CargoResult, bool)>> { let out_dir = self.out_dir(unit); let stem = self.file_stem(unit); @@ -870,3 +861,9 @@ fn env_args(config: &Config, Ok(Vec::new()) } + +impl fmt::Display for Metadata { + fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { + write!(f, "{:016x}", self.0) + } +} diff --git a/src/cargo/ops/cargo_rustc/fingerprint.rs b/src/cargo/ops/cargo_rustc/fingerprint.rs index 6cc8f88c1..952ccac78 100644 --- a/src/cargo/ops/cargo_rustc/fingerprint.rs +++ b/src/cargo/ops/cargo_rustc/fingerprint.rs @@ -531,7 +531,7 @@ pub fn dir(cx: &Context, unit: &Unit) -> PathBuf { } /// Returns the (old, new) location for the dep info file of a target. -pub fn dep_info_loc(cx: &Context, unit: &Unit) -> PathBuf { +pub fn dep_info_loc(cx: &mut Context, unit: &Unit) -> PathBuf { dir(cx, unit).join(&format!("dep-{}", filename(cx, unit))) } @@ -653,7 +653,7 @@ fn mtime_if_fresh(output: &Path, paths: I) -> Option } } -fn filename(cx: &Context, unit: &Unit) -> String { +fn filename(cx: &mut Context, unit: &Unit) -> String { // file_stem includes metadata hash. Thus we have a different // fingerprint for every metadata hash version. This works because // even if the package is fresh, we'll still link the fresh target diff --git a/src/cargo/ops/cargo_rustc/mod.rs b/src/cargo/ops/cargo_rustc/mod.rs index 02e108830..73f3fed96 100644 --- a/src/cargo/ops/cargo_rustc/mod.rs +++ b/src/cargo/ops/cargo_rustc/mod.rs @@ -421,7 +421,7 @@ fn add_plugin_deps(rustc: &mut ProcessBuilder, Ok(()) } -fn prepare_rustc(cx: &Context, +fn prepare_rustc(cx: &mut Context, crate_types: Vec<&str>, unit: &Unit) -> CargoResult { let mut base = cx.compilation.rustc_process(unit.pkg)?; @@ -509,7 +509,7 @@ fn root_path(cx: &Context, unit: &Unit) -> PathBuf { } } -fn build_base_args(cx: &Context, +fn build_base_args(cx: &mut Context, cmd: &mut ProcessBuilder, unit: &Unit, crate_types: &[&str]) { @@ -610,8 +610,8 @@ fn build_base_args(cx: &Context, match cx.target_metadata(unit) { Some(m) => { - cmd.arg("-C").arg(&format!("metadata={}", m.metadata)); - cmd.arg("-C").arg(&format!("extra-filename={}", m.extra_filename)); + cmd.arg("-C").arg(&format!("metadata={}", m)); + cmd.arg("-C").arg(&format!("extra-filename=-{}", m)); } None => { cmd.arg("-C").arg(&format!("metadata={}", short_hash(unit.pkg))); @@ -645,17 +645,16 @@ fn build_plugin_args(cmd: &mut ProcessBuilder, cx: &Context, unit: &Unit) { opt(cmd, "-C", "linker=", cx.linker(unit.kind).map(|s| s.as_ref())); } -fn build_deps_args(cmd: &mut ProcessBuilder, cx: &Context, unit: &Unit) +fn build_deps_args(cmd: &mut ProcessBuilder, cx: &mut Context, unit: &Unit) -> CargoResult<()> { - let layout = cx.layout(unit); cmd.arg("-L").arg(&{ let mut deps = OsString::from("dependency="); - deps.push(layout.deps()); + deps.push(cx.layout(unit).deps()); deps }); if unit.pkg.has_custom_build() { - cmd.env("OUT_DIR", &layout.build_out(unit.pkg)); + cmd.env("OUT_DIR", &cx.layout(unit).build_out(unit.pkg)); } for unit in cx.dep_targets(unit)?.iter() { @@ -666,7 +665,7 @@ fn build_deps_args(cmd: &mut ProcessBuilder, cx: &Context, unit: &Unit) return Ok(()); - fn link_to(cmd: &mut ProcessBuilder, cx: &Context, unit: &Unit) + fn link_to(cmd: &mut ProcessBuilder, cx: &mut Context, unit: &Unit) -> CargoResult<()> { for (dst, _link_dst, linkable) in cx.target_filenames(unit)? { if !linkable { diff --git a/src/cargo/util/toml.rs b/src/cargo/util/toml.rs index c5d70b709..5e5dc93a6 100644 --- a/src/cargo/util/toml.rs +++ b/src/cargo/util/toml.rs @@ -14,7 +14,6 @@ use core::{Summary, Manifest, Target, Dependency, DependencyInner, PackageId}; use core::{EitherManifest, VirtualManifest}; use core::dependency::{Kind, Platform}; use core::manifest::{LibKind, Profile, ManifestMetadata}; -use core::package_id::Metadata; use sources::CRATES_IO; use util::{self, CargoResult, human, ToUrl, ToSemver, ChainError, Config}; @@ -437,7 +436,6 @@ impl TomlManifest { } let pkgid = project.to_package_id(source_id)?; - let metadata = pkgid.generate_metadata(); // If we have no lib at all, use the inferred lib if available // If we have a lib with a path, we're done @@ -550,8 +548,7 @@ impl TomlManifest { new_build, &examples, &tests, - &benches, - &metadata); + &benches); if targets.is_empty() { debug!("manifest has no build targets"); @@ -1064,8 +1061,7 @@ fn normalize(lib: &Option, custom_build: Option, examples: &[TomlExampleTarget], tests: &[TomlTestTarget], - benches: &[TomlBenchTarget], - metadata: &Metadata) -> Vec { + benches: &[TomlBenchTarget]) -> Vec { fn configure(toml: &TomlTarget, target: &mut Target) { let t2 = target.clone(); target.set_tested(toml.test.unwrap_or(t2.tested())) @@ -1080,9 +1076,7 @@ fn normalize(lib: &Option, }); } - fn lib_target(dst: &mut Vec, - l: &TomlLibTarget, - metadata: &Metadata) { + fn lib_target(dst: &mut Vec, l: &TomlLibTarget) { let path = l.path.clone().unwrap_or( PathValue::Path(Path::new("src").join(&format!("{}.rs", l.name()))) ); @@ -1095,14 +1089,8 @@ fn normalize(lib: &Option, } }; - // Binaries, examples, etc, may link to this library. Their crate names - // have a high likelihood to being the same as ours, however, so we need - // some extra metadata in our name to ensure symbols won't collide. - let mut metadata = metadata.clone(); - metadata.mix(&"lib"); let mut target = Target::lib_target(&l.name(), crate_types, - &path.to_path(), - metadata); + &path.to_path()); configure(l, &mut target); dst.push(target); } @@ -1113,8 +1101,7 @@ fn normalize(lib: &Option, let path = bin.path.clone().unwrap_or_else(|| { PathValue::Path(default(bin)) }); - let mut target = Target::bin_target(&bin.name(), &path.to_path(), - None); + let mut target = Target::bin_target(&bin.name(), &path.to_path()); configure(bin, &mut target); dst.push(target); } @@ -1124,7 +1111,7 @@ fn normalize(lib: &Option, let name = format!("build-script-{}", cmd.file_stem().and_then(|s| s.to_str()).unwrap_or("")); - dst.push(Target::custom_build_target(&name, cmd, None)); + dst.push(Target::custom_build_target(&name, cmd)); } fn example_targets(dst: &mut Vec, @@ -1141,40 +1128,29 @@ fn normalize(lib: &Option, } } - fn test_targets(dst: &mut Vec, tests: &[TomlTestTarget], - metadata: &Metadata, + fn test_targets(dst: &mut Vec, + tests: &[TomlTestTarget], default: &mut FnMut(&TomlTestTarget) -> PathBuf) { for test in tests.iter() { let path = test.path.clone().unwrap_or_else(|| { PathValue::Path(default(test)) }); - // make sure this metadata is different from any same-named libs. - let mut metadata = metadata.clone(); - metadata.mix(&format!("test-{}", test.name())); - - let mut target = Target::test_target(&test.name(), &path.to_path(), - metadata); + let mut target = Target::test_target(&test.name(), &path.to_path()); configure(test, &mut target); dst.push(target); } } - fn bench_targets(dst: &mut Vec, benches: &[TomlBenchTarget], - metadata: &Metadata, + fn bench_targets(dst: &mut Vec, + benches: &[TomlBenchTarget], default: &mut FnMut(&TomlBenchTarget) -> PathBuf) { for bench in benches.iter() { let path = bench.path.clone().unwrap_or_else(|| { PathValue::Path(default(bench)) }); - // make sure this metadata is different from any same-named libs. - let mut metadata = metadata.clone(); - metadata.mix(&format!("bench-{}", bench.name())); - - let mut target = Target::bench_target(&bench.name(), - &path.to_path(), - metadata); + let mut target = Target::bench_target(&bench.name(), &path.to_path()); configure(bench, &mut target); dst.push(target); } @@ -1183,7 +1159,7 @@ fn normalize(lib: &Option, let mut ret = Vec::new(); if let Some(ref lib) = *lib { - lib_target(&mut ret, lib, metadata); + lib_target(&mut ret, lib); bin_targets(&mut ret, bins, &mut |bin| Path::new("src").join("bin") .join(&format!("{}.rs", bin.name()))); @@ -1201,7 +1177,7 @@ fn normalize(lib: &Option, &mut |ex| Path::new("examples") .join(&format!("{}.rs", ex.name()))); - test_targets(&mut ret, tests, metadata, &mut |test| { + test_targets(&mut ret, tests, &mut |test| { if test.name() == "test" { Path::new("src").join("test.rs") } else { @@ -1209,7 +1185,7 @@ fn normalize(lib: &Option, } }); - bench_targets(&mut ret, benches, metadata, &mut |bench| { + bench_targets(&mut ret, benches, &mut |bench| { if bench.name() == "bench" { Path::new("src").join("bench.rs") } else { -- 2.30.2